feat: Expand variables as empty in alert queries - #2910

⚠️ No Changeset found

Latest commit: 3b5176a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ignored	Preview	Aug 18, 2026 3:01pm
hyperdx-storybook	Ignored	Preview	Aug 18, 2026 3:01pm

Greptile Summary

The PR makes dashboard-tile alerts evaluate declared variables using empty selections and aligns alert previews and editor messaging with that behavior.

• Centralizes extraction of dashboard variable declarations.
• Injects empty variable selections into scheduled alert queries and alert-detail previews.
• Warns users when an alerting query references dashboard variables.
• Adds unit, integration, and end-to-end coverage for variable expansion and warnings.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Filters[Dashboard variable declarations] --> Empty[Attach empty selections]
  Empty --> Task[Scheduled alert query]
  Empty --> Detail[Alert detail preview]
  Empty --> Editor[Chart editor preview]
  Editor --> Warning[Variable empty-state warning]
  Task --> Query[Shared chart query rendering]
  Detail --> Query
  Editor --> Query

_{Reviews (6): Last reviewed commit: "chore: Update rachet" | Re-trigger Greptile}

E2E Test Results

✅ All tests passed • 300 passed • 1 skipped • 1167s

Tests ran across 4 shards in parallel.

View full report →

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Additional context: touches background tasks or the delivery pipeline lightly (11 lines, under the 30-line bar for Tier 4)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

Deep Review

✅ No critical issues found. The refactor extracting getDashboardVariableDeclarations preserves the prior useDashboardFilters behavior exactly, the alert-execution details.dashboard access is safe (the TILE provider returns early when the dashboard is missing, and AlertDetails.dashboard is non-optional), and substitution failures are caught and recorded as query errors rather than escaping the handler. The findings below are recommended follow-ups, not merge blockers.

🟡 P2 -- recommended

• packages/api/src/tasks/checkAlerts/index.ts:630 -- A bare $var reference in a SQL where/having (e.g. ServiceName = $svc) substitutes an empty selection to the literal NULL, producing an always-false predicate, so the alert matches zero rows every window and silently never fires; unlike $__filter/$__conditionalAll, which correctly become match-all no-ops.
  • Fix: Give bare variable references an empty-state no-op consistent with the macros when running for an alert, or block/hard-warn alert creation on tiles that reference variables via bare non-macro SQL syntax.
  • _{reliability, adversarial}
• packages/app/src/components/alerts/AlertDetailChart.tsx:140 -- The new preview substitution (empty-state variables plus Number→Line display coercion) has no test coverage at any level, so a regression in what the alert preview renders would go undetected.
  • Fix: Add a unit test rendering TileAlertChart with variable-enabled filters and a Number-display tile, asserting the config carries variables with empty values and the display type is coerced to Line.
  • _testing

Reviewers (6): correctness, reliability, adversarial, kieran-typescript, testing, maintainability.

Testing gaps:

• No integration test covers a bare non-macro SQL variable reference (ServiceName = $svc) on an alerting tile — the exact pattern that degrades to = NULL — only the Lucene, $__filter, and $__conditionalAll no-op cases are exercised.
• No coverage for the materialized-view optimization path interacting with variable-bearing alert queries, nor for the 300ms debounce window on the additionalAlertWarnings memo (only eventual outcome is asserted).